Wonderful! I'll look at this closer tonight. That ind2sub method takes a vector of linear indices, and returns a tuple of vectors, with one vector for each dimension. So instead of returning an array of tuples for each index, it returns a tuple of arrays for each dimension. Compare:

julia> map(x->ind2sub((2,3), x), 1:6)
6-element Array{Tuple{Int64,Int64},1}:
 (1,1)
 (2,1)
 (1,2)
 (2,2)
 (1,3)
 (2,3)

julia> ind2sub((2,3), 1:6)
([1,2,1,2,1,2],[1,1,2,2,3,3])

I'm not sure if it's used in base, but I think it'd be useful for some of the sparse array implementations.

Ahah! Thank you for the explanation, Matt.

Looks like you've got some test errors. Can you fix and update the PR? Might be worth running a make testall locally before you do.

Revised (with test for ind2sub added), squashed and rebased. make testall passed on my local machine, so I hope this works.

I'm not familiar with the AppVeyor system. My sense is this build failure appears to be due to a timeout. How ought I to proceed?

It is due to a timeout. The fact that it passed on Travis and Win32 mean there's (probably!) nothing wrong with your tests. @mbauman said he'd take a look in a bit but I see no reason not to merge. Thanks for these great tests :). Someone will owe you a 🍺 very soon.

Yup, looks like a freeze during bootstrap — since this only touched test files it's unrelated.

The only minor comment I have is that I don't think we should define the Base.unsafe_* methods for the tested types. It's currently a little undocumented trick that I'd like to solve in a better way, so I want to make sure that the tests pass without them defined (I just checked - they still do).

This is great, thank you again!

@kshyatt w00t! Glad I could help.

@mbauman That makes sense. For some reason I had been minding the Base.unsafe_* methods as requiring a user-defined fall-back. As that's not the case, I agree that the tested types oughtn't to have such methods defined in the test file. I'll just remove those four method definitions and push again. Also, you're very welcome!

Unrelated: I don't suppose there's any way to make this combination of emojis work in GitHub, is there? http://emoji.octopus.holdings/

🏆
🐙

Folks, please don't merge things when there's a freeze during bootstrap. That is almost always a real problem, and continuing to merge unrelated changes on top of a broken build means additional chances to introduce more platform-specific breakage.

Also, get off my lawn!

Ok, but if we can't restart/requeue the AV run then how are we supposed to check if something is ok?

Anyone with commit access should be able to restart jobs on appveyor. You have to login under the StefanKarpinski account to do so, which is a little confusing.

ref #10579 (comment)

Ok, I got it to work! Thanks @tkelman. Is it ok for me to requeue others that had the timeout if the queue is empty (as it is now)?

I don't expect anything to have changed on master, so I'm not sure what good re-queuing would do right this second.

I was asking more in general. I requeued my PR to figure out how to do it. You can cancel it if you want :)

In general, yeah restarting builds is fine if you think something fell victim to an intermittent problem, better safe than sorry. Especially when appveyor is idle which it should be much more often now that we have 2 concurrent workers.

On-topic for this PR, note that there are deprecation warnings when you run in one worker via make testall1:

     * abstractarray       WARNING: [a] concatenation is deprecated; use collect(a) instead
 in depwarn at ./deprecated.jl:63
 in oldstyle_vcat_warning at ./abstractarray.jl:29
 in test_vcat_depwarn at /home/tkelman/Julia/julia/test/abstractarray.jl:442
 in include at ./boot.jl:254
 in runtests at /home/tkelman/Julia/julia/test/testdefs.jl:197
 in anonymous at multi.jl:629
 in run_work_thunk at multi.jl:590
 in remotecall_fetch at multi.jl:663
 in remotecall_fetch at multi.jl:678
 in anonymous at task.jl:1401
while loading /home/tkelman/Julia/julia/test/abstractarray.jl, in expression starting on line 468
WARNING: [a,b] concatenation is deprecated; use [a;b] instead
 in depwarn at ./deprecated.jl:63
 in oldstyle_vcat_warning at ./abstractarray.jl:29
 in test_vcat_depwarn at /home/tkelman/Julia/julia/test/abstractarray.jl:443
 in include at ./boot.jl:254
 in runtests at /home/tkelman/Julia/julia/test/testdefs.jl:197
 in anonymous at multi.jl:629
 in run_work_thunk at multi.jl:590
 in remotecall_fetch at multi.jl:663
 in remotecall_fetch at multi.jl:678
 in anonymous at task.jl:1401
while loading /home/tkelman/Julia/julia/test/abstractarray.jl, in expression starting on line 468
WARNING: [a,b,...] concatenation is deprecated; use [a;b;...] instead
 in depwarn at ./deprecated.jl:63
 in oldstyle_vcat_warning at ./abstractarray.jl:29
 in test_vcat_depwarn at /home/tkelman/Julia/julia/test/abstractarray.jl:444
 in include at ./boot.jl:254
 in runtests at /home/tkelman/Julia/julia/test/testdefs.jl:197
 in anonymous at multi.jl:629
 in run_work_thunk at multi.jl:590
 in remotecall_fetch at multi.jl:663
 in remotecall_fetch at multi.jl:678
 in anonymous at task.jl:1401
while loading /home/tkelman/Julia/julia/test/abstractarray.jl, in expression starting on line 468

@tkelman That's from testing the vcat deprecation warner: https://github.com/JuliaLang/julia/pull/12086/files#diff-84f49a8bf9fc57eebe9ab5a2cc13a682R436. If, as I have been told, the remote workers have depwarn == 2, then a remote worker oughtn't to have experienced a deprecation warning, correct?

As far as other machines with depwarn == 1 testing with one worker, what do you recommend I change about the test?

In any case, thank you to everybody who's helped me navigate through getting these tests merged (if slightly prematurely)!

@hayd But it's arms aren't held up high enough to make it look like it's holding the trophy! Oh well, I guess that will have to suffice.

I believe when JULIA_CPU_CORES is set to 1, as with make testall1, then remote workers aren't used to run the tests. Could potentially redirect the output or test in a separate process, as is done in a few other places throughout the tests.

@tkelman Would straight up imitating this whole else block do the trick? https://github.com/ScottPJones/julia/blob/2d737744979bf2f3d6a9b6e5ebb3e123c653a097/test/deprecated.jl#L8-L19

I think you'd need the whole thing, not just the else block, so that it works both in the process or on a worker. I was also told that that sort of thing should be in a let block (because of the olderr variable).

@ScottPJones right, I meant replacing the current else block I have with something analogous to yours.

Just checking: the reason a let block would be appropriate here is because if and else blocks don't introduce a local scope, and hence without enclosure in a let block the olderr variable is visible to the rest of the module? Though I don't understand what the harm in that is... or is it just bad house-keeping?

just bad house-keeping?

Mostly this. Every so often a name conflict in dummy test variables will lead to tests failing in funny ways, I actually had to debug exactly this problem just a few days ago. It can depend on which order different workers run which sets of tests, or only fail if you run in a single worker, etc so it's not always obvious what the issue is when this happens.

@tkelman - I figured this one was pretty safe. But you're right, I've triggered more than my fair share of Win64 breakages, and I should have just been patient. I can't wait to give LLVM 3.3 the boot.

@davidagold - sounds like a good plan. Want to make a small PR? Note that since your tests are in a function you don't need to worry about the let block (since they're already scoped)… although I suppose the same sort of collision could happen between two methods of the inadvertently-same-named generic function. That'd be a bear to debug, too.

Is there a reason why we're not just using modules for each test file?

@mbauman Would be happy to!

What in particular about LLVM 3.3 makes the Win64 build seize up? I take it that 3.4 will be better -- when does that get to be incorporated into Julia?

As long as we're able to get back to green without too much wasted time on other PR's, it's alright. I'd really like us to get to a state where master cannot be broken, but lacking the infrastructure to automatically enforce that it'll have to be a cultural thing. Will also be challenging given the number of intermittent failures we seem to hit. http://graydon2.dreamwidth.org/1597.html

Ref #9798 for automatically putting each test in a separate module. That might help the sandboxing if someone wants to pick that up and get it working.

@davidagold see #9336. The biggest issue is a major regression in codegen performance that'll have to be worked out. LLVM upstream is working on 3.7 right now, should be out in August, that'll be the first newer-than-3.3 version we can use that has working backtraces on OSX.

@tkelman Thank you for the resources.

Some of the test function names are pretty generic (like test_map). I'm of the mind to make a token TestAbstractArray type and then amend the methods defined in my PR to take that type as an argument, just to reduce the chance of method conflicts down the road. Thoughts?

Cross-reference to new PR that addresses issues raised above: #12111